Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Brave Ads intermittently crashes when opening a new tab #6304

Merged
merged 1 commit into from
Aug 3, 2020
Merged

Brave Ads intermittently crashes when opening a new tab #6304

merged 1 commit into from
Aug 3, 2020

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jul 31, 2020

Resolves brave/brave-browser#9393

Submitter Checklist:

Test Plan:

  • Confirm ads does not crash if tapping a notification while the browser is shutting down (race condition so may need many tries)

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@tmancey tmancey self-assigned this Jul 31, 2020
@tmancey tmancey force-pushed the issues/9393 branch 2 times, most recently from b04692a to 42e07a2 Compare August 1, 2020 00:01
emerick
emerick approved these changes Aug 1, 2020
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tmancey
Copy link
Collaborator Author

tmancey commented Aug 3, 2020

CI failed due to unrelated gn check and rewards browser tests

@tmancey tmancey merged commit 137588e into master Aug 3, 2020
@tmancey tmancey deleted the issues/9393 branch August 3, 2020 07:15
@tmancey tmancey added this to the 1.14.x - Nightly milestone Aug 3, 2020
This was referenced Aug 5, 2020
Merged
Merged
@kjozwiak
Copy link
Member

kjozwiak commented Aug 6, 2020

Verification PASSED on macOS 10.15.6 x64 using the following build:

Brave | 1.14.21 Chromium: 84.0.4147.105 (Official Build) nightly (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | macOS Version 10.15.6 (Build 19G73)

Ran through the case outlined via #6304 (comment) about ~x15 times and didn't see any crashes being added into brave://crashes. Went through the following:

  • launched 1.14.21 CR: 84.0.4147.105 and enabled rewards
  • visited brave.com and triggered an ad

Once triggered, close Brave via Command + Q and ensured the browser didn't crash by checking brave://crashes. Closed the browser in various stages after clicking an ad notification.

@LaurenWags LaurenWags mentioned this pull request Aug 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@bridiver bridiver Awaiting requested review from bridiver

@NejcZdovc NejcZdovc Awaiting requested review from NejcZdovc

Assignees

@tmancey tmancey

Labels
bug feature/ads
Projects
None yet
Milestone
1.14.x - Release #1
Development

Successfully merging this pull request may close these issues.

Brave Ads intermittently crashes when opening a new tab
3 participants
@tmancey @kjozwiak @emerick